Conversation
connortsui20
added
do not merge
Merging this PR will not alter performance
Comparing 
Footnotes
|
connortsui20
force-pushed
the
ct/sv-array
branch
from
1d55528 to
1ac487c
Compare
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
connortsui20
force-pushed
the
ct/sv-array
branch
from
1ac487c to
1f71c78
Compare
connortsui20
changed the title
ScalarValue::Array
connortsui20
removed
the
do not merge
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
connortsui20
force-pushed
the
ct/sv-array
branch
from
1f71c78 to
b1a66d3
Compare
| /// require passing the length of the array to deserialize with [`ArrayParts::decode()`], we | ||
| /// store the array length in the first 8 bytes. | ||
| /// | ||
| /// [`Array::serialize()`]: crate::Array::serialize |
|
|
||
| // TODO(connor): There is probably performance that is left on the table here. | ||
|
|
||
| impl TryFrom<&[u8]> for ArrayParts { |
| let parts = ArrayParts::try_from(&bytes[8..])?; | ||
|
|
||
| // Deserialize the entire array. | ||
| let ctx = ArrayContext::empty(); |
There was a problem hiding this comment.
This doesn't work. The ArrayContext needs to be the same one that was used at write-time since it contains a mapping of u16 -> EncodingId.
I think we're better off using the IPC crate which already adds a header for framing that includes the context + the length
There was a problem hiding this comment.
We can't depend on vortex-ipc since it already has a dependency on vortex-array. I can mimic it in a similar way though.
2 participants
Summary
Adds an
Arrayvariant toScalarValuethat holds anArrayRef.This also requires passing a
VortexSessionin a lot more places (this is why we needed to do #6751, #6759, and #6761).API Changes
TODO
Testing
TODO
A lot of things have to be updated still...